-
-
Notifications
You must be signed in to change notification settings - Fork 481
Fix CLI crash when using custom config (dark: false) #1649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix CLI crash when using custom config (dark: false) #1649
Conversation
🦋 Changeset detectedLatest commit: c62e967 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@aliamadi is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughReplaced a simple truth-check in the init logger's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Please provide full reproduction steps, because the "could crash" is very vague and non technical, also I never experienced this locally or in any React based framework or technology out of all specified in the "Integrations". It either crashes or not under certain situations, which I would like to test myself before working on this. |
|
Thanks for the extra info. This is exactly what I wanted to highlight out of this, the node version has something to do with it and I need to dig into how it resolves the JS part in that exact version. |
|
I see it now, can confirm the issue. eg: get showWarning() {
let hasChecked = false;
for (const value of this.checkedMap.values()) {
if (value) {
hasChecked = true;
break;
}
}
return !hasChecked;
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/cli/utils/create-init-logger.ts (1)
23-32: Consider renaminghasCheckedfor clarity and verify if size guard is needed.The variable name
hasCheckedis misleading—it sounds like "has checked any files" but actually means "found a ThemeInit component." Consider renaming tofoundThemeInitorhasThemeInitfor clarity.Additionally, the PR description mentions adding a guard expression (
this.checkedMap.size > 0 && ...) to only evaluate the warning logic after at least one file has been checked. The current implementation will returntrue(show warning) whencheckedMapis empty, which means the warning displays even if no files have been processed yet. Was omitting the size check intentional based on reviewer feedback, or should it be added?🔎 Suggested improvements
Option 1: Clearer naming only
- let hasChecked = false; + let foundThemeInit = false; for (const value of this.checkedMap.values()) { if (value) { - hasChecked = true; + foundThemeInit = true; break; } } - return !hasChecked; + return !foundThemeInit;Option 2: With size guard (as mentioned in PR description)
- let hasChecked = false; + if (this.checkedMap.size === 0) { + return false; + } + + let foundThemeInit = false; for (const value of this.checkedMap.values()) { if (value) { - hasChecked = true; + foundThemeInit = true; break; } } - return !hasChecked; + return !foundThemeInit;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/twelve-dogs-refuse.mdpackages/ui/src/cli/utils/create-init-logger.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SutuSebastian
Repo: themesberg/flowbite-react PR: 1621
File: packages/ui/src/cli/commands/install.ts:46-48
Timestamp: 2025-09-01T10:45:00.071Z
Learning: In the flowbite-react CLI install command, the maintainer prefers a simple Windows support implementation that appends .cmd to all commands on win32, rather than a more defensive approach, because they know the command will only be package managers from the resolveCommand function.
📚 Learning: 2025-09-01T10:45:00.071Z
Learnt from: SutuSebastian
Repo: themesberg/flowbite-react PR: 1621
File: packages/ui/src/cli/commands/install.ts:46-48
Timestamp: 2025-09-01T10:45:00.071Z
Learning: In the flowbite-react CLI install command, the maintainer prefers a simple Windows support implementation that appends .cmd to all commands on win32, rather than a more defensive approach, because they know the command will only be package managers from the resolveCommand function.
Applied to files:
.changeset/twelve-dogs-refuse.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 🧰 Build
🔇 Additional comments (1)
.changeset/twelve-dogs-refuse.md (1)
1-5: LGTM!The changeset format is correct, the patch bump is appropriate for this bug fix, and the description clearly summarizes the change.
Thanks! I replaced the logic with the suggested for...of loop |
SutuSebastian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!

Summary
The Flowbite React CLI could crash at runtime when a custom configuration
was used (e.g.
dark: false). This triggers the CLI to evaluate the<ThemeInit />warning logic, where an array method was incorrectly calledon a
MapIterator.Fix
checkedMapvalues without calling array methods on aMapIteratorVerification
Built the package and tested it in a Next.js project where the CLI previously
crashed with
dark: false. The issue no longer occurs.Summary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.